Skip to content

fix: add retry logic for channel monitor retrieval during migration#424

Merged
jvsena42 merged 2 commits intomasterfrom
fix/channel-monitor-silent-failure
Feb 5, 2026
Merged

fix: add retry logic for channel monitor retrieval during migration#424
jvsena42 merged 2 commits intomasterfrom
fix/channel-monitor-silent-failure

Conversation

@jvsena42
Copy link
Member

@jvsena42 jvsena42 commented Feb 2, 2026

Description

This PR fixes a critical bug where channel monitor retrieval failures during migration from the React Native app were silently dropped, potentially causing permanent loss of Lightning channel state.

  1. Adds retrieveChannelMonitorWithRetry method with configurable retry attempts and linear backoff
  2. Implements parallel retrieval with proper error tracking for each channel
  3. Logs individual failures with channel IDs for debugging
  4. Validates expected vs retrieved monitor count with warnings

This is the iOS equivalent of the Android fix: synonymdev/bitkit-android#760

Linked Issues/Tasks

Related: synonymdev/bitkit-android#760

Screenshot / Video

N/A - internal migration logic fix

@jvsena42
Copy link
Member Author

jvsena42 commented Feb 2, 2026

@jvsena42 jvsena42 self-assigned this Feb 2, 2026
@claude

This comment has been minimized.

@jvsena42 jvsena42 requested review from ben-kaufman and pwltr February 2, 2026 17:02
@pwltr
Copy link
Contributor

pwltr commented Feb 3, 2026

Is it intentional that we continue with restoring if one or more channel monitors fail to retrieve? Seems to me like we should only restore if everything goes right, and if not show the restore error.

@jvsena42
Copy link
Member Author

jvsena42 commented Feb 3, 2026

Is it intentional that we continue with restoring if one or more channel monitors fail to retrieve? Seems to me like we should only restore if everything goes right, and if not show the restore error.

I thought that if an error happens for some reason that we don't know yet, other than just connection, the user could be stuck outside the wallet

On the case that we got from support, the wallet migrate almost all the channels except one, that was force closed

But I agree, the restore error is also a valid solution, since the user could also try to restore from mnemonics if they are backed up

@pwltr
Copy link
Contributor

pwltr commented Feb 3, 2026

It might be okay to continue if the channel_manager restores even if all of the channel_monitors fail if that's what we want to do, at least then the channels will force close and the user will get their funds. But continuing without channel_manager seems problematic, since the user will not see their LN balance without any further information (I believe, haven't tested). The design also has UI for this case, which I think is not implemented:

image

Copy link
Contributor

@pwltr pwltr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK, comments above not applicable to this PR.

@jvsena42
Copy link
Member Author

jvsena42 commented Feb 3, 2026

But continuing without channel_manager seems problematic, since the user will not see their LN balance without any further information (I believe, haven't tested)

Indeed, that is what happened. I found it was a force close from the logs

@jvsena42
Copy link
Member Author

jvsena42 commented Feb 3, 2026

Waiting for the migration tests to succeed

https://github.com/synonymdev/bitkit-ios/actions/runs/21628335768

@jvsena42 jvsena42 enabled auto-merge February 4, 2026 21:24
@jvsena42 jvsena42 merged commit 34b8074 into master Feb 5, 2026
37 of 66 checks passed
@jvsena42 jvsena42 deleted the fix/channel-monitor-silent-failure branch February 5, 2026 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants